Skip to content

Conversation

ldematte
Copy link
Contributor

We were missing a couple of cases where File.createTempFile (from java.io) are defaulting to the default temp directory.
This PR addresses that.

Fixes #130086

@ldematte ldematte requested a review from a team as a code owner August 26, 2025 09:27
@ldematte ldematte added >non-issue auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure v9.2.0 v9.1.4 v8.19.4 labels Aug 26, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Aug 26, 2025
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC I didn't do this because we implicitly allow access to the temp directory for all code, so I only overrode the methods that took in a base dir to create a temp file. This should work though, it just means additional checks (we already grant the tmp dir access automatically in module policies, do we also do that for eg the unnamed module that apm agent uses?)

@ldematte
Copy link
Contributor Author

It's a good point; at first, I just added a

if (directory != null) {
   // check write to directory. Otherwise skip as we always allow access to the temp directory
}

case to the existing createTempFile overload.

What I did not like about it is that we now have the "we implicitly allow access to the temp directory for all code" policy in yet another place, so I change it this way.
It is a bit more clean IMO as this keeps the policy we have is (I think) in a single place, coded in FileAccessTree.

I'm fine either way really, even just adding the directory != null check.

@ldematte
Copy link
Contributor Author

we already grant the tmp dir access automatically in module policies, do we also do that for eg the unnamed module that apm agent uses?

Yep, all paths (including APM_MODULE and UNKNOWN) all use a FileAccessTree. They might have a defaultEntitlements / defaultFileAccess (with FilesEntitlement.EMPTY), but we add special permissions like temp or config access in the FileAccessTree ctor.

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ldematte ldematte enabled auto-merge (squash) August 27, 2025 07:34
@ldematte ldematte merged commit a4e3b0a into elastic:main Aug 27, 2025
34 checks passed
ldematte added a commit to ldematte/elasticsearch that referenced this pull request Aug 27, 2025
We were missing a couple of cases where File.createTempFile (from java.io) are defaulting to the default temp directory.
This PR addresses that.

Fixes elastic#130086
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.1
8.19

ldematte added a commit to ldematte/elasticsearch that referenced this pull request Aug 27, 2025
We were missing a couple of cases where File.createTempFile (from java.io) are defaulting to the default temp directory.
This PR addresses that.

Fixes elastic#130086
elasticsearchmachine pushed a commit that referenced this pull request Aug 27, 2025
We were missing a couple of cases where File.createTempFile (from java.io) are defaulting to the default temp directory.
This PR addresses that.

Fixes #130086
elasticsearchmachine pushed a commit that referenced this pull request Aug 27, 2025
We were missing a couple of cases where File.createTempFile (from java.io) are defaulting to the default temp directory.
This PR addresses that.

Fixes #130086
sarog pushed a commit to portsbuild/elasticsearch that referenced this pull request Sep 11, 2025
…c#133647)

We were missing a couple of cases where File.createTempFile (from java.io) are defaulting to the default temp directory.
This PR addresses that.

Fixes elastic#130086
sarog pushed a commit to portsbuild/elasticsearch that referenced this pull request Sep 19, 2025
…c#133647)

We were missing a couple of cases where File.createTempFile (from java.io) are defaulting to the default temp directory.
This PR addresses that.

Fixes elastic#130086
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure >non-issue Team:Core/Infra Meta label for core/infra team v8.19.4 v9.1.4 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to use File.createTempFile in custom plugin because the new entitlement limitation

3 participants